Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Authorize by GitHub org and or team membership #205

Merged
merged 21 commits into from
Mar 12, 2020
Merged

Authorize by GitHub org and or team membership #205

merged 21 commits into from
Mar 12, 2020

Conversation

eikehartmann
Copy link
Contributor

Allows configuration for GitHub to set whitelist on teams and or organizations when configured like this:

vouch:
  teamWhitelist:
   - myOrg
   - myOrg/myTeam

As mentioned in #150 this requires additional GitHub API calls to check the membership status.

Also includes the fix for checks of an email against domains: #199.

@bnfinet
Copy link
Member

bnfinet commented Feb 7, 2020

nice stuff @eikehartmann thanks for the contribution. Both @aaronpk and myself are very keen to get this in.

Could you make just a couple adjustments...

  • move GitHub specific logic to its own file (in the handlers package or as a separate package). handlers.go is getting a bit crowded
  • can you consider the GitHub enterprise (on prem) use case, how do you configure the api call?

And thanks for adding such thorough tests to VP

@eikehartmann
Check GitHub user team memberships and store them in User struct
5507bd6
@eikehartmann
Evaluate team memberships when verifying user
7b0f0a3
@eikehartmann
Remover Org from config and use <orgId>/<teamSlug> as format for gith…
0b99ff8 
…ub team whitelist
@eikehartmann
Add test assertions on urls called
fd72962
@eikehartmann
Add org membership url to config and github defaults
4ad1f13
@eikehartmann
Add method to check for github org membership
4a9d20a
@eikehartmann
Check for GitHub Org membership if no team qualified in TeamWhiteList…
6ad9c84 
… value

 - i.e. only "<orgId>" and not fully qualified team "<orgId>/<teamSlug>" is given in config's teamWhitelist
@eikehartmann
Add documentation for teamWhitelist to github example config
227cd5b
@eikehartmann
Move Github-related handler stuff to own package
fd1019a
@eikehartmann
Move IndieAuth-related handler stuff to own package
cfb5be2
@eikehartmann
Move ADFS-related handler stuff to own package
82f7c18
@eikehartmann
Move HomeAssistant-related handler stuff to own package
eacccd5
@eikehartmann
Move OpenStax-related handler stuff to own package
d367704
@eikehartmann
Move Google-related handler stuff to own package
2b6659d
@eikehartmann
Move OpenID-related handler stuff to own package
fb73f16
@eikehartmann
Refactor to common parameters for different vendor methods
02dd4da
@eikehartmann
Use strategy pattern-like switch to select vendor-specific handler
1244cbd
@eikehartmann eikehartmann changed the title Authorize by GitHub org and or team membership WIP: Authorize by GitHub org and or team membership Feb 10, 2020
@eikehartmann
Copy link
Contributor Author

Hi @bnfinet,

thanks for the review and input. I moved the vendor-specific stuff to their own packages and tried to generalise on the function call. GitHub enterprise sample config has also been updated.

Will still improve logging and error handling today and remove the WIP label once done.

@eikehartmann eikehartmann changed the title WIP: Authorize by GitHub org and or team membership Authorize by GitHub org and or team membership Feb 10, 2020
@bnfinet
Copy link
Member

bnfinet commented Feb 11, 2020

much appreciated @eikehartmann

I am engaged to another project at the moment and won't be able to give this my attention for a few more days. I am hopeful of this week but it may slip to next week. Thanks for the code and your patience.

@bnfinet bnfinet merged commit 8dce88f into vouch:master Mar 12, 2020
@bnfinet
Copy link
Member

bnfinet commented Mar 12, 2020

@eikehartmann thanks again for the fine work!

@bnfinet bnfinet mentioned this pull request Mar 12, 2020
Merged
eikehartmann
eikehartmann commented Mar 12, 2020
View reviewed changes
pkg/domains/domains.go
log.Debugf("removing port from %s to test domain %s", s, split[0])
s = split[0]
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

bnfinet added a commit that referenced this pull request Apr 8, 2020
@bnfinet
#205 err should be returned at the end
7069684
bnfinet added a commit that referenced this pull request May 22, 2020
@bnfinet
#205 err should be returned at the end
a5ef36d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@eikehartmann @bnfinet